-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update admins input UI #533
Conversation
Hey @ori-near, please take a look at the UI and let me know if it works as expected |
Hi @Megha-Dev-19 – this looks awesome! A few follow up requests:
|
Hey @ori-near |
Hi @Megha-Dev-19 – we want to avoid a situation where a group is left without any admins. Essentially whoever creates the group to begin with is the default admin. They can't remove themselves unless they add a secondary admin. Let's say you are the admin of a page you created. You won't be able to remove yourself, but if you add me, I can remove you or you can remove yourself. But the last person cannot be removed unless they add someone else. |
We have a prevention on contract side, so if you're removing the last admin, contract would detect it and prevent you from doing so. Of course, It would be nice to also have that validation on frontend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides what @ori-near mentioned issues, this PR looks good to me!
Hi @Megha-Dev-19 – thanks for updating this. Latest feedback below. ✅ indicates good to go, ❌ indicates remaining issue to address.
|
Hi @Megha-Dev-19 – this looks great, nicely done. I just have one last additional request:
With 5 done, I consider this ticket accepted 🚀 and ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me as well. Thank you @Megha-Dev-19 !
Resolves #241
Preview link